feat(tracing): process tags about service name source [APMLP-1052]#16593
feat(tracing): process tags about service name source [APMLP-1052]#16593emmettbutler wants to merge 13 commits intomainfrom
Conversation
emmettbutler
added
the
changelog/no-changelog
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e6c92b4d54
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if not config.enabled: | ||
| return None, None | ||
|
|
||
| from ddtrace import config as ddtrace_config |
There was a problem hiding this comment.
ddtrace.config during process-tags init
When DD_EXPERIMENTAL_PROPAGATE_PROCESS_TAGS_ENABLED=true, generate_process_tags() executes at module import time and hits from ddtrace import config; at that point ddtrace/__init__.py has imported telemetry (which imports process_tags) before it binds config, so this creates a circular import and can raise ImportError: cannot import name 'config' from partially initialized module 'ddtrace', preventing startup whenever process tags are enabled from env.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I don't think this is true. The test I added proves it.
There was a problem hiding this comment.
So we have a problem because, tests are passing but codex is right, if you run
DD_EXPERIMENTAL_PROPAGATE_PROCESS_TAGS_ENABLED=True ddtrace-run python your_script.py, you indeed have a circular import (that is also why I created a config for process_tags).
There was a problem hiding this comment.
Ah yes. This arises from the fact that process tags are computed before/during ddtrace configs and they also need to include information from those configs.
This comment has been minimized.
This comment has been minimized.
Performance SLOsComparing candidate emmett.butler/svc-process-tags (ad7f9f1) with baseline main (8c9f2b5) 📈 Performance Regressions (2 suites)📈 iastaspects - 118/118✅ add_aspectTime: ✅ 103.621µs (SLO: <130.000µs 📉 -20.3%) vs baseline: +0.5% Memory: ✅ 43.018MB (SLO: <46.000MB -6.5%) vs baseline: +4.8% ✅ add_inplace_aspectTime: ✅ 100.904µs (SLO: <130.000µs 📉 -22.4%) vs baseline: -3.4% Memory: ✅ 43.018MB (SLO: <46.000MB -6.5%) vs baseline: +4.6% ✅ add_inplace_noaspectTime: ✅ 28.413µs (SLO: <40.000µs 📉 -29.0%) vs baseline: ~same Memory: ✅ 42.900MB (SLO: <46.000MB -6.7%) vs baseline: +4.2% ✅ add_noaspectTime: ✅ 48.776µs (SLO: <70.000µs 📉 -30.3%) vs baseline: -1.0% Memory: ✅ 42.939MB (SLO: <46.000MB -6.7%) vs baseline: +4.4% ✅ bytearray_aspectTime: ✅ 250.336µs (SLO: <400.000µs 📉 -37.4%) vs baseline: +0.5% Memory: ✅ 42.920MB (SLO: <46.000MB -6.7%) vs baseline: +4.5% ✅ bytearray_extend_aspectTime: ✅ 639.676µs (SLO: <800.000µs 📉 -20.0%) vs baseline: -2.4% Memory: ✅ 42.979MB (SLO: <46.000MB -6.6%) vs baseline: +4.4% ✅ bytearray_extend_noaspectTime: ✅ 264.529µs (SLO: <400.000µs 📉 -33.9%) vs baseline: -2.5% Memory: ✅ 42.939MB (SLO: <46.000MB -6.7%) vs baseline: +4.4% ✅ bytearray_noaspectTime: ✅ 136.437µs (SLO: <300.000µs 📉 -54.5%) vs baseline: -2.6% Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +4.3% ✅ bytes_aspectTime: ✅ 218.741µs (SLO: <300.000µs 📉 -27.1%) vs baseline: -0.6% Memory: ✅ 42.998MB (SLO: <46.000MB -6.5%) vs baseline: +4.5% ✅ bytes_noaspectTime: ✅ 132.205µs (SLO: <200.000µs 📉 -33.9%) vs baseline: -1.8% Memory: ✅ 42.939MB (SLO: <46.000MB -6.7%) vs baseline: +4.5% ✅ bytesio_aspectTime: ✅ 3.768ms (SLO: <5.000ms 📉 -24.6%) vs baseline: -1.4% Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +4.4% ✅ bytesio_noaspectTime: ✅ 313.631µs (SLO: <420.000µs 📉 -25.3%) vs baseline: -1.2% Memory: ✅ 42.861MB (SLO: <46.000MB -6.8%) vs baseline: +4.2% ✅ capitalize_aspectTime: ✅ 89.438µs (SLO: <300.000µs 📉 -70.2%) vs baseline: +0.9% Memory: ✅ 42.861MB (SLO: <46.000MB -6.8%) vs baseline: +4.3% ✅ capitalize_noaspectTime: ✅ 250.317µs (SLO: <300.000µs 📉 -16.6%) vs baseline: -2.8% Memory: ✅ 42.939MB (SLO: <46.000MB -6.7%) vs baseline: +4.8% ✅ casefold_aspectTime: ✅ 89.369µs (SLO: <500.000µs 📉 -82.1%) vs baseline: +0.5% Memory: ✅ 42.900MB (SLO: <46.000MB -6.7%) vs baseline: +4.1% ✅ casefold_noaspectTime: ✅ 304.274µs (SLO: <500.000µs 📉 -39.1%) vs baseline: -2.2% Memory: ✅ 42.939MB (SLO: <46.000MB -6.7%) vs baseline: +4.5% ✅ decode_aspectTime: ✅ 86.805µs (SLO: <100.000µs 📉 -13.2%) vs baseline: ~same Memory: ✅ 42.880MB (SLO: <46.000MB -6.8%) vs baseline: +4.4% ✅ decode_noaspectTime: ✅ 152.044µs (SLO: <210.000µs 📉 -27.6%) vs baseline: -1.3% Memory: ✅ 42.939MB (SLO: <46.000MB -6.7%) vs baseline: +4.5% ✅ encode_aspectTime: ✅ 83.983µs (SLO: <200.000µs 📉 -58.0%) vs baseline: -0.6% Memory: ✅ 42.920MB (SLO: <46.000MB -6.7%) vs baseline: +4.6% ✅ encode_noaspectTime: ✅ 140.711µs (SLO: <200.000µs 📉 -29.6%) vs baseline: -1.3% Memory: ✅ 42.939MB (SLO: <46.000MB -6.7%) vs baseline: +4.6% ✅ format_aspectTime: ✅ 14.611ms (SLO: <19.200ms 📉 -23.9%) vs baseline: ~same Memory: ✅ 42.979MB (SLO: <46.000MB -6.6%) vs baseline: +4.2% ✅ format_map_aspectTime: ✅ 16.413ms (SLO: <21.500ms 📉 -23.7%) vs baseline: ~same Memory: ✅ 43.037MB (SLO: <46.000MB -6.4%) vs baseline: +4.1% ✅ format_map_noaspectTime: ✅ 374.312µs (SLO: <500.000µs 📉 -25.1%) vs baseline: -0.7% Memory: ✅ 42.939MB (SLO: <46.000MB -6.7%) vs baseline: +4.4% ✅ format_noaspectTime: ✅ 302.502µs (SLO: <500.000µs 📉 -39.5%) vs baseline: -1.6% Memory: ✅ 42.920MB (SLO: <46.000MB -6.7%) vs baseline: +4.6% ✅ index_aspectTime: ✅ 124.153µs (SLO: <300.000µs 📉 -58.6%) vs baseline: +1.7% Memory: ✅ 42.900MB (SLO: <46.000MB -6.7%) vs baseline: +4.4% ✅ index_noaspectTime: ✅ 40.457µs (SLO: <300.000µs 📉 -86.5%) vs baseline: -0.6% Memory: ✅ 42.900MB (SLO: <46.000MB -6.7%) vs baseline: +4.1% ✅ join_aspectTime: ✅ 210.187µs (SLO: <300.000µs 📉 -29.9%) vs baseline: -2.2% Memory: ✅ 42.900MB (SLO: <46.000MB -6.7%) vs baseline: +4.4% ✅ join_noaspectTime: ✅ 141.306µs (SLO: <300.000µs 📉 -52.9%) vs baseline: -2.2% Memory: ✅ 42.939MB (SLO: <46.000MB -6.7%) vs baseline: +4.4% ✅ ljust_aspectTime: ✅ 586.168µs (SLO: <700.000µs 📉 -16.3%) vs baseline: 📈 +15.5% Memory: ✅ 42.920MB (SLO: <46.000MB -6.7%) vs baseline: +4.3% ✅ ljust_noaspectTime: ✅ 258.587µs (SLO: <300.000µs 📉 -13.8%) vs baseline: -2.2% Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +4.2% ✅ lower_aspectTime: ✅ 294.944µs (SLO: <500.000µs 📉 -41.0%) vs baseline: -2.6% Memory: ✅ 42.979MB (SLO: <46.000MB -6.6%) vs baseline: +4.6% ✅ lower_noaspectTime: ✅ 235.111µs (SLO: <300.000µs 📉 -21.6%) vs baseline: -0.2% Memory: ✅ 42.939MB (SLO: <46.000MB -6.7%) vs baseline: +4.3% ✅ lstrip_aspectTime: ✅ 0.269ms (SLO: <3.000ms 📉 -91.0%) vs baseline: -2.7% Memory: ✅ 42.939MB (SLO: <46.000MB -6.7%) vs baseline: +4.2% ✅ lstrip_noaspectTime: ✅ 0.177ms (SLO: <3.000ms 📉 -94.1%) vs baseline: -2.5% Memory: ✅ 42.920MB (SLO: <46.000MB -6.7%) vs baseline: +4.0% ✅ modulo_aspectTime: ✅ 14.291ms (SLO: <18.750ms 📉 -23.8%) vs baseline: +0.2% Memory: ✅ 43.096MB (SLO: <46.000MB -6.3%) vs baseline: +4.6% ✅ modulo_aspect_for_bytearray_bytearrayTime: ✅ 14.776ms (SLO: <19.350ms 📉 -23.6%) vs baseline: ~same Memory: ✅ 43.077MB (SLO: <46.000MB -6.4%) vs baseline: +4.5% ✅ modulo_aspect_for_bytesTime: ✅ 14.350ms (SLO: <18.900ms 📉 -24.1%) vs baseline: -0.1% Memory: ✅ 42.979MB (SLO: <46.000MB -6.6%) vs baseline: +4.3% ✅ modulo_aspect_for_bytes_bytearrayTime: ✅ 14.626ms (SLO: <19.150ms 📉 -23.6%) vs baseline: +0.2% Memory: ✅ 43.018MB (SLO: <46.000MB -6.5%) vs baseline: +4.5% ✅ modulo_noaspectTime: ✅ 0.359ms (SLO: <3.000ms 📉 -88.0%) vs baseline: -1.1% Memory: ✅ 42.920MB (SLO: <46.000MB -6.7%) vs baseline: +4.2% ✅ replace_aspectTime: ✅ 18.396ms (SLO: <24.000ms 📉 -23.3%) vs baseline: -0.2% Memory: ✅ 43.018MB (SLO: <46.000MB -6.5%) vs baseline: +4.4% ✅ replace_noaspectTime: ✅ 279.006µs (SLO: <300.000µs -7.0%) vs baseline: -1.3% Memory: ✅ 42.880MB (SLO: <46.000MB -6.8%) vs baseline: +4.0% ✅ repr_aspectTime: ✅ 310.254µs (SLO: <420.000µs 📉 -26.1%) vs baseline: -3.0% Memory: ✅ 42.979MB (SLO: <46.000MB -6.6%) vs baseline: +4.7% ✅ repr_noaspectTime: ✅ 46.561µs (SLO: <90.000µs 📉 -48.3%) vs baseline: -0.8% Memory: ✅ 42.920MB (SLO: <46.000MB -6.7%) vs baseline: +4.4% ✅ rstrip_aspectTime: ✅ 381.708µs (SLO: <500.000µs 📉 -23.7%) vs baseline: -0.6% Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +4.6% ✅ rstrip_noaspectTime: ✅ 185.956µs (SLO: <300.000µs 📉 -38.0%) vs baseline: +1.9% Memory: ✅ 42.939MB (SLO: <46.000MB -6.7%) vs baseline: +4.4% ✅ slice_aspectTime: ✅ 183.986µs (SLO: <300.000µs 📉 -38.7%) vs baseline: +1.6% Memory: ✅ 42.979MB (SLO: <46.000MB -6.6%) vs baseline: +4.3% ✅ slice_noaspectTime: ✅ 53.953µs (SLO: <90.000µs 📉 -40.1%) vs baseline: -0.2% Memory: ✅ 42.920MB (SLO: <46.000MB -6.7%) vs baseline: +4.3% ✅ stringio_aspectTime: ✅ 3.837ms (SLO: <5.000ms 📉 -23.3%) vs baseline: -1.2% Memory: ✅ 42.920MB (SLO: <46.000MB -6.7%) vs baseline: +4.4% ✅ stringio_noaspectTime: ✅ 379.693µs (SLO: <500.000µs 📉 -24.1%) vs baseline: +5.7% Memory: ✅ 42.900MB (SLO: <46.000MB -6.7%) vs baseline: +4.6% ✅ strip_aspectTime: ✅ 271.841µs (SLO: <350.000µs 📉 -22.3%) vs baseline: -2.4% Memory: ✅ 42.939MB (SLO: <46.000MB -6.7%) vs baseline: +4.3% ✅ strip_noaspectTime: ✅ 178.658µs (SLO: <240.000µs 📉 -25.6%) vs baseline: -0.7% Memory: ✅ 42.979MB (SLO: <46.000MB -6.6%) vs baseline: +4.4% ✅ swapcase_aspectTime: ✅ 337.032µs (SLO: <500.000µs 📉 -32.6%) vs baseline: -1.2% Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +4.4% ✅ swapcase_noaspectTime: ✅ 273.089µs (SLO: <400.000µs 📉 -31.7%) vs baseline: -1.1% Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +4.4% ✅ title_aspectTime: ✅ 318.270µs (SLO: <500.000µs 📉 -36.3%) vs baseline: -3.0% Memory: ✅ 42.920MB (SLO: <46.000MB -6.7%) vs baseline: +4.4% ✅ title_noaspectTime: ✅ 258.302µs (SLO: <400.000µs 📉 -35.4%) vs baseline: -1.7% Memory: ✅ 42.900MB (SLO: <46.000MB -6.7%) vs baseline: +4.6% ✅ translate_aspectTime: ✅ 493.176µs (SLO: <700.000µs 📉 -29.5%) vs baseline: -1.5% Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +4.4% ✅ translate_noaspectTime: ✅ 423.736µs (SLO: <500.000µs 📉 -15.3%) vs baseline: -2.1% Memory: ✅ 42.900MB (SLO: <46.000MB -6.7%) vs baseline: +4.2% ✅ upper_aspectTime: ✅ 296.117µs (SLO: <500.000µs 📉 -40.8%) vs baseline: -0.2% Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +4.8% ✅ upper_noaspectTime: ✅ 237.056µs (SLO: <400.000µs 📉 -40.7%) vs baseline: +0.3% Memory: ✅ 42.880MB (SLO: <46.000MB -6.8%) vs baseline: +4.2% 📈 iastaspectsospath - 24/24✅ ospathbasename_aspectTime: ✅ 510.358µs (SLO: <700.000µs 📉 -27.1%) vs baseline: 📈 +17.2% Memory: ✅ 42.920MB (SLO: <46.000MB -6.7%) vs baseline: +5.1% ✅ ospathbasename_noaspectTime: ✅ 431.178µs (SLO: <700.000µs 📉 -38.4%) vs baseline: -2.4% Memory: ✅ 42.979MB (SLO: <46.000MB -6.6%) vs baseline: +5.4% ✅ ospathjoin_aspectTime: ✅ 624.776µs (SLO: <700.000µs 📉 -10.7%) vs baseline: +1.0% Memory: ✅ 42.920MB (SLO: <46.000MB -6.7%) vs baseline: +4.7% ✅ ospathjoin_noaspectTime: ✅ 637.147µs (SLO: <700.000µs -9.0%) vs baseline: +0.4% Memory: ✅ 42.723MB (SLO: <46.000MB -7.1%) vs baseline: +4.9% ✅ ospathnormcase_aspectTime: ✅ 348.177µs (SLO: <700.000µs 📉 -50.3%) vs baseline: -2.2% Memory: ✅ 42.939MB (SLO: <46.000MB -6.7%) vs baseline: +5.4% ✅ ospathnormcase_noaspectTime: ✅ 357.508µs (SLO: <700.000µs 📉 -48.9%) vs baseline: -1.4% Memory: ✅ 42.979MB (SLO: <46.000MB -6.6%) vs baseline: +5.6% ✅ ospathsplit_aspectTime: ✅ 492.409µs (SLO: <700.000µs 📉 -29.7%) vs baseline: -0.1% Memory: ✅ 42.979MB (SLO: <46.000MB -6.6%) vs baseline: +5.4% ✅ ospathsplit_noaspectTime: ✅ 501.268µs (SLO: <700.000µs 📉 -28.4%) vs baseline: +0.7% Memory: ✅ 42.703MB (SLO: <46.000MB -7.2%) vs baseline: +4.7% ✅ ospathsplitdrive_aspectTime: ✅ 372.599µs (SLO: <700.000µs 📉 -46.8%) vs baseline: +0.5% Memory: ✅ 42.880MB (SLO: <46.000MB -6.8%) vs baseline: +5.3% ✅ ospathsplitdrive_noaspectTime: ✅ 72.388µs (SLO: <700.000µs 📉 -89.7%) vs baseline: -0.9% Memory: ✅ 42.880MB (SLO: <46.000MB -6.8%) vs baseline: +5.1% ✅ ospathsplitext_aspectTime: ✅ 456.577µs (SLO: <700.000µs 📉 -34.8%) vs baseline: -1.0% Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +5.1% ✅ ospathsplitext_noaspectTime: ✅ 466.230µs (SLO: <700.000µs 📉 -33.4%) vs baseline: -0.8% Memory: ✅ 42.959MB (SLO: <46.000MB -6.6%) vs baseline: +5.1% 🟡 Near SLO Breach (1 suite)🟡 tracer - 6/6✅ largeTime: ✅ 31.698ms (SLO: <32.950ms -3.8%) vs baseline: +0.9% Memory: ✅ 36.825MB (SLO: <39.250MB -6.2%) vs baseline: +4.6% ✅ mediumTime: ✅ 3.104ms (SLO: <3.200ms -3.0%) vs baseline: +0.1% Memory: ✅ 35.547MB (SLO: <38.750MB -8.3%) vs baseline: +4.6% ✅ smallTime: ✅ 364.641µs (SLO: <370.000µs 🟡 -1.4%) vs baseline: +4.7% Memory: ✅ 35.586MB (SLO: <38.750MB -8.2%) vs baseline: +4.5%
|
Codeowners resolved as |
dubloom
left a comment
dubloom
left a comment
There was a problem hiding this comment.
There is indeed a circular import.
Otherwise, LGTM
| if not config.enabled: | ||
| return None, None | ||
|
|
||
| from ddtrace import config as ddtrace_config |
There was a problem hiding this comment.
So we have a problem because, tests are passing but codex is right, if you run
DD_EXPERIMENTAL_PROPAGATE_PROCESS_TAGS_ENABLED=True ddtrace-run python your_script.py, you indeed have a circular import (that is also why I created a config for process_tags).
emmettbutler
removed
the
changelog/no-changelog
emmettbutler
requested review from
dubloom,
rachelyangdog and
vlad-scherbich
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ad7f9f10ef
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| base_hash, base_hash_bytes = None, b"" | ||
| process_tags, process_tags_list = generate_process_tags() | ||
| process_tags = None |
There was a problem hiding this comment.
This change replaces eager module initialization with process_tags = None, but TraceTagsProcessor.process_trace and profiler startup still read process_tags.process_tags without calling _set_globals(). In configurations where process-tag propagation is enabled but no early caller initializes tags first (for example when telemetry/remote-config paths are disabled), spans stop getting _dd.tags.process and dependent hash propagation stays unset, which regresses behavior that previously worked because tags were generated at import time.
Useful? React with 👍 / 👎.
2 participants
This change implements the RFC referenced in AMPLP-1052 describing a process-level tagging structure indicating the source of the service name.